-
Notifications
You must be signed in to change notification settings - Fork 15.6k
[clang-tidy] Add fine-grained options to misc-use-internal-linkage
#173310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
✅ With the latest revision this PR passed the RST documentation linter. |
20ceeec to
5b70e98
Compare
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
5b70e98 to
1cbadfb
Compare
1cbadfb to
421ae7b
Compare
|
@llvm/pr-subscribers-clang-tidy Author: Victor Chernyakin (localspook) ChangesThere were several requests in #172797 to add fine-grained configuration for this check: here it is. By default, everything is enabled. Full diff: https://github.com/llvm/llvm-project/pull/173310.diff 9 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
index 0bfb5cd52353a..617b98484f3e4 100644
--- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
@@ -109,10 +109,22 @@ UseInternalLinkageCheck::UseInternalLinkageCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
HeaderFileExtensions(Context->getHeaderFileExtensions()),
- FixMode(Options.get("FixMode", FixModeKind::UseStatic)) {}
+ FixMode(Options.get("FixMode", FixModeKind::UseStatic)),
+ AnalyzeFunctions(Options.get("AnalyzeFunctions", true)),
+ AnalyzeVariables(Options.get("AnalyzeVariables", true)),
+ AnalyzeTypes(Options.get("AnalyzeTypes", true)) {
+ if (!AnalyzeFunctions && !AnalyzeVariables && !AnalyzeTypes)
+ configurationDiag(
+ "the 'misc-use-internal-linkage' check will not perform any "
+ "analysis because its 'AnalyzeFunctions', 'AnalyzeVariables', "
+ "and 'AnalyzeTypes' options have all been set to false");
+}
void UseInternalLinkageCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "FixMode", FixMode);
+ Options.store(Opts, "AnalyzeFunctions", AnalyzeFunctions);
+ Options.store(Opts, "AnalyzeVariables", AnalyzeVariables);
+ Options.store(Opts, "AnalyzeTypes", AnalyzeTypes);
}
void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
@@ -125,24 +137,26 @@ void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
friendDecl(),
// 3. module export decl
exportDecl()))))));
- Finder->addMatcher(
- functionDecl(Common, hasBody(),
- unless(anyOf(isExternC(), isStaticStorageClass(),
- isExternStorageClass(),
- isExplicitTemplateSpecialization(),
- cxxMethodDecl(), isConsteval(),
- isAllocationOrDeallocationOverloadedFunction(),
- isMain())))
- .bind("fn"),
- this);
- Finder->addMatcher(varDecl(Common, hasGlobalStorage(),
- unless(anyOf(isExternC(), isStaticStorageClass(),
- isExternStorageClass(),
- isExplicitTemplateSpecialization(),
- hasThreadStorageDuration())))
- .bind("var"),
- this);
- if (getLangOpts().CPlusPlus)
+ if (AnalyzeFunctions)
+ Finder->addMatcher(
+ functionDecl(
+ Common, hasBody(),
+ unless(anyOf(
+ isExternC(), isStaticStorageClass(), isExternStorageClass(),
+ isExplicitTemplateSpecialization(), cxxMethodDecl(),
+ isConsteval(), isAllocationOrDeallocationOverloadedFunction(),
+ isMain())))
+ .bind("fn"),
+ this);
+ if (AnalyzeVariables)
+ Finder->addMatcher(varDecl(Common, hasGlobalStorage(),
+ unless(anyOf(isExternC(), isStaticStorageClass(),
+ isExternStorageClass(),
+ isExplicitTemplateSpecialization(),
+ hasThreadStorageDuration())))
+ .bind("var"),
+ this);
+ if (getLangOpts().CPlusPlus && AnalyzeTypes)
Finder->addMatcher(
tagDecl(Common, isDefinition(), hasNameForLinkage(),
hasDeclContext(anyOf(translationUnitDecl(), namespaceDecl())),
diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h
index eedb5bd1adcab..5ab3eae515a0f 100644
--- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h
+++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h
@@ -36,6 +36,9 @@ class UseInternalLinkageCheck : public ClangTidyCheck {
private:
FileExtensionsSet HeaderFileExtensions;
FixModeKind FixMode;
+ const bool AnalyzeFunctions;
+ const bool AnalyzeVariables;
+ const bool AnalyzeTypes;
};
} // namespace clang::tidy::misc
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e4ff640811933..5bf2177ed5941 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -519,7 +519,9 @@ Changes in existing checks
- Improved :doc:`misc-use-internal-linkage
<clang-tidy/checks/misc/use-internal-linkage>` to suggest giving
- structs, classes, unions, and enums internal linkage.
+ user-defined types (structs, classes, unions, and enums) internal
+ linkage. Added fine-grained options to control whether the check
+ should diagnose functions, variables, and/or user-defined types.
- Improved :doc:`modernize-avoid-c-arrays
<clang-tidy/checks/modernize/avoid-c-arrays>` to not diagnose array types
diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
index 8838837506a40..da094fd5ec0e0 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
@@ -17,7 +17,7 @@ Example:
int v1; // can be marked as static
void fn1() {} // can be marked as static
-
+
struct S1 {}; // can be moved into anonymous namespace
namespace {
@@ -50,3 +50,16 @@ Options
- `UseStatic`
Add ``static`` for internal linkage variable and function.
+
+.. option:: AnalyzeFunctions
+
+ Whether to suggest giving functions internal linkage. Default is `true`.
+
+.. option:: AnalyzeVariables
+
+ Whether to suggest giving variables internal linkage. Default is `true`.
+
+.. option:: AnalyzeTypes
+
+ (C++-only) Whether to suggest giving user-defined types (structs,
+ classes, unions, and enums) internal linkage. Default is `true`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/tag.h b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/type.h
similarity index 100%
rename from clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/tag.h
rename to clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/type.h
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp
index 1709ebc5f7144..d25e4383613f7 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp
@@ -1,6 +1,15 @@
-// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage
// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
-// RUN: -config="{CheckOptions: {misc-use-internal-linkage.FixMode: 'UseStatic'}}" -- -I%S/Inputs/use-internal-linkage
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-use-internal-linkage.AnalyzeVariables: false, \
+// RUN: misc-use-internal-linkage.AnalyzeTypes: false \
+// RUN: }}" -- -I%S/Inputs/use-internal-linkage
+
+// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-use-internal-linkage.FixMode: 'UseStatic', \
+// RUN: misc-use-internal-linkage.AnalyzeVariables: false, \
+// RUN: misc-use-internal-linkage.AnalyzeTypes: false \
+// RUN: }}" -- -I%S/Inputs/use-internal-linkage
#include "func.h"
@@ -57,7 +66,6 @@ NNDS void func_nnds() {}
void func_h_inc() {}
struct S {
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: struct 'S' can be moved into an anonymous namespace to enforce internal linkage
void method();
};
void S::method() {}
@@ -96,3 +104,6 @@ void * operator new[](std::size_t) { return nullptr; }
void operator delete(void*) noexcept {}
void operator delete[](void*) noexcept {}
// gh117489 end
+
+int ignored_global = 0; // AnalyzeVariables is false.
+struct IgnoredStruct {}; // AnalyzeTypes is false.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-tag.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-type.cpp
similarity index 88%
rename from clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-tag.cpp
rename to clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-type.cpp
index 55bdfd4ca36c6..771d7e9d422c4 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-tag.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-type.cpp
@@ -1,8 +1,15 @@
-// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage
// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
-// RUN: -config="{CheckOptions: {misc-use-internal-linkage.FixMode: 'UseStatic'}}" -- -I%S/Inputs/use-internal-linkage
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-use-internal-linkage.AnalyzeFunctions: false \
+// RUN: }}" -- -I%S/Inputs/use-internal-linkage
-#include "tag.h"
+// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-use-internal-linkage.FixMode: 'UseStatic', \
+// RUN: misc-use-internal-linkage.AnalyzeFunctions: false \
+// RUN: }}" -- -I%S/Inputs/use-internal-linkage
+
+#include "type.h"
struct StructDeclaredInHeader {};
union UnionDeclaredInHeader {};
@@ -56,7 +63,6 @@ struct OuterStruct {
struct OuterStruct::InnerStructDefinedOutOfLine {};
void f() {
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' can be made static or moved into an anonymous namespace to enforce internal linkage
struct StructInsideFunction {};
}
@@ -97,3 +103,5 @@ extern "C" {
struct InExternCBlock { int i; };
}
+
+void ignored_func() {} // AnalyzeFunctions is false.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp
index e30fc7b54953e..1be7165f9ffe6 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp
@@ -1,6 +1,15 @@
-// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage
// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
-// RUN: -config="{CheckOptions: {misc-use-internal-linkage.FixMode: 'UseStatic'}}" -- -I%S/Inputs/use-internal-linkage
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-use-internal-linkage.AnalyzeFunctions: false, \
+// RUN: misc-use-internal-linkage.AnalyzeTypes: false \
+// RUN: }}" -- -I%S/Inputs/use-internal-linkage
+
+// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-use-internal-linkage.FixMode: 'UseStatic', \
+// RUN: misc-use-internal-linkage.AnalyzeFunctions: false, \
+// RUN: misc-use-internal-linkage.AnalyzeTypes: false \
+// RUN: }}" -- -I%S/Inputs/use-internal-linkage
#include "var.h"
@@ -47,7 +56,6 @@ static void f(int para) {
}
struct S {
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: struct 'S' can be moved into an anonymous namespace to enforce internal linkage
int m1;
static int m2;
};
@@ -61,3 +69,6 @@ extern "C" int global_in_extern_c_2;
const int const_global = 123;
constexpr int constexpr_global = 123;
+
+struct IgnoredStruct {}; // AnalyzeTypes is false.
+void ignored_func() {} // AnalyzeFunctions is false.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-wrong-config.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-wrong-config.cpp
new file mode 100644
index 0000000000000..a99f17e5ddc0a
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-wrong-config.cpp
@@ -0,0 +1,8 @@
+// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-use-internal-linkage.AnalyzeFunctions: false, \
+// RUN: misc-use-internal-linkage.AnalyzeVariables: false, \
+// RUN: misc-use-internal-linkage.AnalyzeTypes: false \
+// RUN: }}"
+
+// CHECK-MESSAGES: warning: the 'misc-use-internal-linkage' check will not perform any analysis because its 'AnalyzeFunctions', 'AnalyzeVariables', and 'AnalyzeTypes' options have all been set to false [clang-tidy-config]
|
|
@llvm/pr-subscribers-clang-tools-extra Author: Victor Chernyakin (localspook) ChangesThere were several requests in #172797 to add fine-grained configuration for this check: here it is. By default, everything is enabled. Full diff: https://github.com/llvm/llvm-project/pull/173310.diff 9 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
index 0bfb5cd52353a..617b98484f3e4 100644
--- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp
@@ -109,10 +109,22 @@ UseInternalLinkageCheck::UseInternalLinkageCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
HeaderFileExtensions(Context->getHeaderFileExtensions()),
- FixMode(Options.get("FixMode", FixModeKind::UseStatic)) {}
+ FixMode(Options.get("FixMode", FixModeKind::UseStatic)),
+ AnalyzeFunctions(Options.get("AnalyzeFunctions", true)),
+ AnalyzeVariables(Options.get("AnalyzeVariables", true)),
+ AnalyzeTypes(Options.get("AnalyzeTypes", true)) {
+ if (!AnalyzeFunctions && !AnalyzeVariables && !AnalyzeTypes)
+ configurationDiag(
+ "the 'misc-use-internal-linkage' check will not perform any "
+ "analysis because its 'AnalyzeFunctions', 'AnalyzeVariables', "
+ "and 'AnalyzeTypes' options have all been set to false");
+}
void UseInternalLinkageCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "FixMode", FixMode);
+ Options.store(Opts, "AnalyzeFunctions", AnalyzeFunctions);
+ Options.store(Opts, "AnalyzeVariables", AnalyzeVariables);
+ Options.store(Opts, "AnalyzeTypes", AnalyzeTypes);
}
void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
@@ -125,24 +137,26 @@ void UseInternalLinkageCheck::registerMatchers(MatchFinder *Finder) {
friendDecl(),
// 3. module export decl
exportDecl()))))));
- Finder->addMatcher(
- functionDecl(Common, hasBody(),
- unless(anyOf(isExternC(), isStaticStorageClass(),
- isExternStorageClass(),
- isExplicitTemplateSpecialization(),
- cxxMethodDecl(), isConsteval(),
- isAllocationOrDeallocationOverloadedFunction(),
- isMain())))
- .bind("fn"),
- this);
- Finder->addMatcher(varDecl(Common, hasGlobalStorage(),
- unless(anyOf(isExternC(), isStaticStorageClass(),
- isExternStorageClass(),
- isExplicitTemplateSpecialization(),
- hasThreadStorageDuration())))
- .bind("var"),
- this);
- if (getLangOpts().CPlusPlus)
+ if (AnalyzeFunctions)
+ Finder->addMatcher(
+ functionDecl(
+ Common, hasBody(),
+ unless(anyOf(
+ isExternC(), isStaticStorageClass(), isExternStorageClass(),
+ isExplicitTemplateSpecialization(), cxxMethodDecl(),
+ isConsteval(), isAllocationOrDeallocationOverloadedFunction(),
+ isMain())))
+ .bind("fn"),
+ this);
+ if (AnalyzeVariables)
+ Finder->addMatcher(varDecl(Common, hasGlobalStorage(),
+ unless(anyOf(isExternC(), isStaticStorageClass(),
+ isExternStorageClass(),
+ isExplicitTemplateSpecialization(),
+ hasThreadStorageDuration())))
+ .bind("var"),
+ this);
+ if (getLangOpts().CPlusPlus && AnalyzeTypes)
Finder->addMatcher(
tagDecl(Common, isDefinition(), hasNameForLinkage(),
hasDeclContext(anyOf(translationUnitDecl(), namespaceDecl())),
diff --git a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h
index eedb5bd1adcab..5ab3eae515a0f 100644
--- a/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h
+++ b/clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.h
@@ -36,6 +36,9 @@ class UseInternalLinkageCheck : public ClangTidyCheck {
private:
FileExtensionsSet HeaderFileExtensions;
FixModeKind FixMode;
+ const bool AnalyzeFunctions;
+ const bool AnalyzeVariables;
+ const bool AnalyzeTypes;
};
} // namespace clang::tidy::misc
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e4ff640811933..5bf2177ed5941 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -519,7 +519,9 @@ Changes in existing checks
- Improved :doc:`misc-use-internal-linkage
<clang-tidy/checks/misc/use-internal-linkage>` to suggest giving
- structs, classes, unions, and enums internal linkage.
+ user-defined types (structs, classes, unions, and enums) internal
+ linkage. Added fine-grained options to control whether the check
+ should diagnose functions, variables, and/or user-defined types.
- Improved :doc:`modernize-avoid-c-arrays
<clang-tidy/checks/modernize/avoid-c-arrays>` to not diagnose array types
diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
index 8838837506a40..da094fd5ec0e0 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc/use-internal-linkage.rst
@@ -17,7 +17,7 @@ Example:
int v1; // can be marked as static
void fn1() {} // can be marked as static
-
+
struct S1 {}; // can be moved into anonymous namespace
namespace {
@@ -50,3 +50,16 @@ Options
- `UseStatic`
Add ``static`` for internal linkage variable and function.
+
+.. option:: AnalyzeFunctions
+
+ Whether to suggest giving functions internal linkage. Default is `true`.
+
+.. option:: AnalyzeVariables
+
+ Whether to suggest giving variables internal linkage. Default is `true`.
+
+.. option:: AnalyzeTypes
+
+ (C++-only) Whether to suggest giving user-defined types (structs,
+ classes, unions, and enums) internal linkage. Default is `true`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/tag.h b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/type.h
similarity index 100%
rename from clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/tag.h
rename to clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-internal-linkage/type.h
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp
index 1709ebc5f7144..d25e4383613f7 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp
@@ -1,6 +1,15 @@
-// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage
// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
-// RUN: -config="{CheckOptions: {misc-use-internal-linkage.FixMode: 'UseStatic'}}" -- -I%S/Inputs/use-internal-linkage
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-use-internal-linkage.AnalyzeVariables: false, \
+// RUN: misc-use-internal-linkage.AnalyzeTypes: false \
+// RUN: }}" -- -I%S/Inputs/use-internal-linkage
+
+// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-use-internal-linkage.FixMode: 'UseStatic', \
+// RUN: misc-use-internal-linkage.AnalyzeVariables: false, \
+// RUN: misc-use-internal-linkage.AnalyzeTypes: false \
+// RUN: }}" -- -I%S/Inputs/use-internal-linkage
#include "func.h"
@@ -57,7 +66,6 @@ NNDS void func_nnds() {}
void func_h_inc() {}
struct S {
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: struct 'S' can be moved into an anonymous namespace to enforce internal linkage
void method();
};
void S::method() {}
@@ -96,3 +104,6 @@ void * operator new[](std::size_t) { return nullptr; }
void operator delete(void*) noexcept {}
void operator delete[](void*) noexcept {}
// gh117489 end
+
+int ignored_global = 0; // AnalyzeVariables is false.
+struct IgnoredStruct {}; // AnalyzeTypes is false.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-tag.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-type.cpp
similarity index 88%
rename from clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-tag.cpp
rename to clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-type.cpp
index 55bdfd4ca36c6..771d7e9d422c4 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-tag.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-type.cpp
@@ -1,8 +1,15 @@
-// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage
// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
-// RUN: -config="{CheckOptions: {misc-use-internal-linkage.FixMode: 'UseStatic'}}" -- -I%S/Inputs/use-internal-linkage
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-use-internal-linkage.AnalyzeFunctions: false \
+// RUN: }}" -- -I%S/Inputs/use-internal-linkage
-#include "tag.h"
+// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-use-internal-linkage.FixMode: 'UseStatic', \
+// RUN: misc-use-internal-linkage.AnalyzeFunctions: false \
+// RUN: }}" -- -I%S/Inputs/use-internal-linkage
+
+#include "type.h"
struct StructDeclaredInHeader {};
union UnionDeclaredInHeader {};
@@ -56,7 +63,6 @@ struct OuterStruct {
struct OuterStruct::InnerStructDefinedOutOfLine {};
void f() {
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' can be made static or moved into an anonymous namespace to enforce internal linkage
struct StructInsideFunction {};
}
@@ -97,3 +103,5 @@ extern "C" {
struct InExternCBlock { int i; };
}
+
+void ignored_func() {} // AnalyzeFunctions is false.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp
index e30fc7b54953e..1be7165f9ffe6 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp
@@ -1,6 +1,15 @@
-// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage
// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
-// RUN: -config="{CheckOptions: {misc-use-internal-linkage.FixMode: 'UseStatic'}}" -- -I%S/Inputs/use-internal-linkage
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-use-internal-linkage.AnalyzeFunctions: false, \
+// RUN: misc-use-internal-linkage.AnalyzeTypes: false \
+// RUN: }}" -- -I%S/Inputs/use-internal-linkage
+
+// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-use-internal-linkage.FixMode: 'UseStatic', \
+// RUN: misc-use-internal-linkage.AnalyzeFunctions: false, \
+// RUN: misc-use-internal-linkage.AnalyzeTypes: false \
+// RUN: }}" -- -I%S/Inputs/use-internal-linkage
#include "var.h"
@@ -47,7 +56,6 @@ static void f(int para) {
}
struct S {
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: struct 'S' can be moved into an anonymous namespace to enforce internal linkage
int m1;
static int m2;
};
@@ -61,3 +69,6 @@ extern "C" int global_in_extern_c_2;
const int const_global = 123;
constexpr int constexpr_global = 123;
+
+struct IgnoredStruct {}; // AnalyzeTypes is false.
+void ignored_func() {} // AnalyzeFunctions is false.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-wrong-config.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-wrong-config.cpp
new file mode 100644
index 0000000000000..a99f17e5ddc0a
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-wrong-config.cpp
@@ -0,0 +1,8 @@
+// RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- \
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-use-internal-linkage.AnalyzeFunctions: false, \
+// RUN: misc-use-internal-linkage.AnalyzeVariables: false, \
+// RUN: misc-use-internal-linkage.AnalyzeTypes: false \
+// RUN: }}"
+
+// CHECK-MESSAGES: warning: the 'misc-use-internal-linkage' check will not perform any analysis because its 'AnalyzeFunctions', 'AnalyzeVariables', and 'AnalyzeTypes' options have all been set to false [clang-tidy-config]
|
| @@ -1,8 +1,15 @@ | |||
| // RUN: %check_clang_tidy %s misc-use-internal-linkage %t -- -- -I%S/Inputs/use-internal-linkage | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally named this file ...-tag.cpp because it was testing TagDecls, but I've renamed it to ...-type.cpp to match the option AnalyzeTypes.
zeyi2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, thanks
|
|
||
| .. option:: AnalyzeTypes | ||
|
|
||
| (C++-only) Whether to suggest giving user-defined types (structs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I'm not sure if (C++-only) is the preferred wording here. I grepped the codebase and found that (C++ only) (without the hyphen) is much more common in comments (e.g. clang/lib/Sema/SemaType.cpp, clang/include/clang/AST/Decl.h).
Maybe we could simply drop the hyphen or switch to natural language similar to modernize-unary-static-assert
vbvictor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm minus open comment
There were several requests in #172797 to add fine-grained configuration for this check: here it is. By default, everything is enabled.
Note: I think the tests could be better expressed if they were all in the same file and used combinations of
-check-suffixes={VAR,FUNC,TYPE}, but I imagine merging all the tests would have made reviewing confusing. Maybe a future PR?